fix(mappers): timeout go list in the Go mapper - #194
Conversation
Pass timeoutMs through runCommandArgs so a wedged go list cannot hang clawpatch map. Default is 2 minutes (CLAWPATCH_GO_LIST_TIMEOUT_MS). On timeout, mapping falls back to the existing file walk. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs maintainer review before merge. Reviewed August 31, 2026, 8:26 PM ET / September 1, 2026, 00:26 UTC. ClawSweeper reviewWhat this changesThe branch gives Go package discovery a configurable deadline with file-based fallback and includes the shared Windows command-timeout cleanup needed to preserve that deadline. Merge readinessKeep open. Current main still waits indefinitely for a stalled Priority: P1 Review scores
Verification
How this fits togetherClawpatch’s mapper examines repository files and language tooling to produce feature seeds for later workflow planning. The Go mapper invokes flowchart LR
A[Repository root] --> B[Go mapper]
B --> C[Go list discovery]
C --> D{Completes before deadline?}
D -->|Yes| E[Package feature seeds]
D -->|No| F[File-based package discovery]
F --> E
E --> G[Workflow feature plan]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the shared executor correction in #195 first, then land this focused Go-mapper timeout and fallback behavior with its documented override. Do we have a high-confidence way to reproduce the issue? Yes—current main’s direct child wait has no timeout, and the supplied real CLI proof uses a hanging Is this the best way to solve the issue? Yes. Reusing the existing command runner and existing file fallback is the narrowest maintainable repair, while discarding timed-out partial output prevents incomplete package metadata from being trusted. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 09900a5edd2b. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Include the shared Windows executor fix from openclaw#195 so Go mapping has bounded cleanup. Verify hanging Go invocation and discard incomplete package output before falling back to files. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
|
Maintainer follow-up: the remaining fixes are implemented on this branch at The Go override now rejects unsupported timer values, and the tests verify real shim invocation and discard partial output before file-based fallback. The shared Windows executor correction from #195 is included unchanged, so timeout cleanup is bounded here too. Built CLI proof: with Final CI passed Linux and Windows jobs, including typecheck, lint, formatting, full tests, build, and packaged CLI smoke. Native Windows executor plus Go tests passed (28 passed, 1 Unix-only skip), and the eight Go regression tests passed locally with a larger startup allowance for loaded hosts. Codex autoreview returned scoped-clean at the default P0 threshold. LAND recommended after #195. A merge simulation confirms the two branches combine without conflicts; no PR was merged or closed. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
2 participants
clawpatch mapcould hang indefinitely whengo list ./...stalled. Go discovery now uses the shared command runner with a two-minute deadline (CLAWPATCH_GO_LIST_TIMEOUT_MS). On timeout, it discards incomplete output and uses the existing file-based package mapper.The override accepts 1 through 2147483647 milliseconds, truncates fractional values, and falls back to two minutes for invalid values. This branch includes the shared Windows executor correction from #195, so a hung
taskkillcannot make cleanup unbounded. Land #195 first; its executor code is identical here, and the remaining Go changes are arranged to merge cleanly afterward.Regression tests verify that the hanging tool actually starts, that partial output is discarded, that fallback still emits
Go package store, and that unsupported timer values fall back safely. The test deadline leaves time for process startup on loaded machines. Configuration docs and the changelog are updated; @SebTardif's original contribution and credit are preserved.Built CLI proof used a synthetic Go module and a verified hanging
goexecutable. On macOS, the original mapper remained hung at the four-second outer deadline. The corrected mapper returned in 1762ms, exit 0, and produced two features includingGo package store. Native Windows returned in 1964ms with the same result and confirmed executable invocation.Focused Go tests pass on macOS (8 tests); native Windows executor and Go tests pass (28 passed, 1 Unix-only skip). The branch's GitHub CI runs typecheck, lint, formatting, the full suite, build, packaged CLI smoke, and Windows executor tests. Codex autoreview returned scoped-clean at the default P0 threshold.